fix(dtypes,strings): never abort cleaning on an unusual object cell - #453
Merged
Merged
Conversation
Three crashes on the default fd.clean path, all from code that assumed
every cell in an object column is scalar text:
* non-UTF-8 bytes: dtype inference cast a sample with astype("string"),
and pandas 2 decodes bytes when casting to StringDtype, so a BLOB read
out of a database raised UnicodeDecodeError. pandas 1.5 returned the
frame with the cell untouched. Text inspection now falls back to a view
that keeps str cells and ignores the rest, so both versions leave the
cell alone.
* pd.NA beside a list or dict: the strip and case passes counted repairs
with stripped.ne(s), whose flex comparison hands object columns to
NumPy, which calls bool() on pd.NA != pd.NA and raises "boolean value
of NA is ambiguous". Only str cells can change, so the comparison is
restricted to those positions.
* booleans mixed with pd.NaT: BooleanArray accepts only None/NaN as a
missing value, so [None, None, NaT, False] raised TypeError("Need to
pass bool-like values") -- but only when the frame had another column,
because that changed inference order. Missing cells are normalized to
NaN before the boolean cast.
Closes #447
Closes #448
Closes #451
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
FreshData benchmark report —
|
| fixture | n_rows | n_cols | p50 s | p95 s | peak MB | repair % | false-repair % | preserve % | trust | monotonic | export % |
|---|
Authored-code reduction (Metric 6)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three crashes on the default
fd.cleanpath, found by fuzzing. Each aborts the whole call on data that arrives routinely from databases, JSON and spreadsheets.UnicodeDecodeErroron pandas 2pd.NAbeside a list or dict raisesTypeError: boolean value of NA is ambiguouspd.NaTraiseTypeError: Need to pass bool-like valuesThey share one assumption: that every cell of an object column is scalar text.
Root cause
astype("string"). pandas 2 decodes bytes when casting toStringDtype, so one non-UTF-8 byte aborts the cast — a BLOB read straight out of a database is enough. pandas 1.5 left such a cell untouched, so the two versions disagreed.stripped.ne(s). That flex comparison hands object columns to NumPy, which callsbool()onpd.NA != pd.NAand raises. A list or dict cell keeps the column object-dtype, which is exactly how JSON-derived data looks, so[pd.NA, []]was enough. Every API that runs the pipeline was affected:clean,explain_clean,compare_cleanandclean_enterprise.BooleanArrayaccepts onlyNone/NaNas a missing value, so apd.NaTraised even though the caller had already treated it as missing. It only triggered when the frame had another column, because that changed dtype-inference order and made the single-column case escape.Behaviour change
strcells and ignores everything else, so an undecodable cell is left exactly as it was, on both pandas versions. ASCII bytes are unaffected.str. Lists, dicts, sets and tuples are never rewritten and never counted, and the reported repair count no longer includes cells the pipeline did not touch.NaT: missing cells are normalized toNaNbefore the boolean cast, so the real booleans still convert and the missing cells stay missing.Default-output changes
None for frames that already cleaned successfully.
tests/test_golden.pypasses untouched).Tests
tests/test_dtypes.py: an undecodable bytes cell survives while the rest of the frame cleans; ASCII bytes columns are unchanged; booleans mixed withNaTclean both with and without a second column (the inference-order trigger); ordinary boolean columns still convert.tests/test_strings.py:pd.NAbeside a list cell cleans; list, dict, set and tuple cells are passed through while ordinary text is still stripped; repair counts exclude untouched container cells.Verification
ruff checkandmypy src/freshdata: clean.main.tests/test_dtypes.py,tests/test_strings.pyandtests/test_golden.py: 111 passed on py3.12, 108 passed / 3 skipped on py3.9.not online and not largelanes: py3.12 6514 passed, 17 skipped; py3.9 6485 passed, 21 skipped.